Set isrecursive to false when listall=true by default for users by davidjumani · Pull Request #6111 · apache/cloudstack

davidjumani · 2022-03-14T07:58:08Z

Description

Sets isrecursive=false for users when listall=true by default to prevent any permission issues when listing resources as a user

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

nvazquez · 2022-03-14T16:26:40Z

@blueorangutan package

blueorangutan · 2022-03-14T16:27:03Z

@nvazquez a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2022-03-14T17:15:34Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2870

nvazquez · 2022-03-14T17:20:31Z

@blueorangutan test

blueorangutan · 2022-03-14T17:22:03Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2022-03-15T01:49:28Z

Trillian test result (tid-3599)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29067 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6111-t3599-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py

davidjumani · 2022-03-15T04:20:47Z

@blueorangutan test

blueorangutan · 2022-03-15T04:21:03Z

@davidjumani a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

This reverts commit 38366c8.

davidjumani · 2022-03-15T11:05:20Z

@blueorangutan package

blueorangutan · 2022-03-15T11:07:03Z

@davidjumani a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

nvazquez

Code LGTM

blueorangutan · 2022-03-15T11:44:27Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2885

nvazquez · 2022-03-15T11:47:55Z

@blueorangutan test

blueorangutan · 2022-03-15T11:49:03Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

sureshanaparti

code LGTM

Pearl1594 · 2022-03-15T13:34:25Z

-            return recursive == null ? true : recursive;
+            Account caller = CallContext.current().getCallingAccount();
+            if (caller.getType() != Account.Type.NORMAL) {
+                return recursive == null ? true : recursive;


Could be simplified to

Suggested change

return recursive == null ? true : recursive;

return recursive == null || recursive;

more concise, less readable /m no me gusta

Pearl1594 · 2022-03-15T13:34:26Z

+                return recursive == null ? true : recursive;
+            }
        }
        return recursive == null ? false : recursive;


Could probably be simplified to:

Suggested change

return recursive == null ? false : recursive;

return recursive != null && recursive;

hm, that is not more expressive, i'd say.

Pearl1594

LGTM - verified fix - Prior fix few APIs failed with the following execption:
Only ROOT admins and Domain admins can list service offerings with isrecursive=true

blueorangutan · 2022-03-15T21:42:14Z

Trillian test result (tid-3613)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34034 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6111-t3613-kvm-centos7.zip
Smoke tests completed. 87 look OK, 5 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_add_primary_storage_disabled_host	`Error`	0.63	test_primary_storage.py
test_01_primary_storage_nfs	`Error`	0.11	test_primary_storage.py
ContextSuite context=TestStorageTags>:setup	`Error`	0.19	test_primary_storage.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	29.86	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	60.50	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	40.13	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	0.05	test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	78.75	test_kubernetes_clusters.py
test_01_secure_vm_migration	`Error`	151.49	test_vm_life_cycle.py
test_02_unsecure_vm_migration	`Error`	267.94	test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration	`Error`	142.87	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	43.70	test_vm_life_cycle.py
test_02_list_snapshots_with_removed_data_store	`Error`	9.43	test_snapshots.py
test_02_list_snapshots_with_removed_data_store	`Error`	9.44	test_snapshots.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	303.73	test_hostha_kvm.py

davidjumani · 2022-03-16T04:19:35Z

@blueorangutan test

blueorangutan · 2022-03-16T04:20:03Z

@davidjumani a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

davidjumani · 2022-03-16T06:32:06Z

Closing in favour of #6125

Fix recursive lookup for offerings for users

38366c8

nvazquez added this to the 4.17.0.0 milestone Mar 14, 2022

davidjumani added 2 commits March 15, 2022 16:23

Revert "Fix recursive lookup for offerings for users"

35ca308

This reverts commit 38366c8.

Set isrecursive as false by default of users

16f653d

davidjumani changed the title ~~Fix recursive lookup for offerings for users~~ Set isrecursive to false when listall=true by default for users Mar 15, 2022

nvazquez approved these changes Mar 15, 2022

View reviewed changes

nvazquez added the component:api label Mar 15, 2022

nvazquez requested review from Pearl1594 and weizhouapache March 15, 2022 11:18

sureshanaparti approved these changes Mar 15, 2022

View reviewed changes

Pearl1594 reviewed Mar 15, 2022

View reviewed changes

davidjumani closed this Mar 16, 2022

yadvr removed this from the 4.17.0.0 milestone Mar 24, 2022

	return recursive == null ? true : recursive;
	return recursive == null \|\| recursive;

	return recursive == null ? false : recursive;
	return recursive != null && recursive;

Uh oh!

Conversation

davidjumani commented Mar 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

nvazquez commented Mar 14, 2022

Uh oh!

blueorangutan commented Mar 14, 2022

Uh oh!

blueorangutan commented Mar 14, 2022

Uh oh!

nvazquez commented Mar 14, 2022

Uh oh!

blueorangutan commented Mar 14, 2022

Uh oh!

blueorangutan commented Mar 15, 2022

Uh oh!

davidjumani commented Mar 15, 2022

Uh oh!

blueorangutan commented Mar 15, 2022

Uh oh!

davidjumani commented Mar 15, 2022

Uh oh!

blueorangutan commented Mar 15, 2022

Uh oh!

nvazquez left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Mar 15, 2022

Uh oh!

nvazquez commented Mar 15, 2022

Uh oh!

blueorangutan commented Mar 15, 2022

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

Pearl1594 Mar 15, 2022

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Mar 15, 2022

Choose a reason for hiding this comment

Uh oh!

Pearl1594 Mar 15, 2022

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Mar 15, 2022

Choose a reason for hiding this comment

Uh oh!

Pearl1594 left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Mar 15, 2022

Uh oh!

davidjumani commented Mar 16, 2022

Uh oh!

blueorangutan commented Mar 16, 2022

Uh oh!

davidjumani commented Mar 16, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

davidjumani commented Mar 14, 2022 •

edited

Loading